Skip to content

W25/tony/deprecate o auth#58

Closed
tonyqiu123 wants to merge 7 commits intomainfrom
W25/tony/deprecate-oAuth
Closed

W25/tony/deprecate o auth#58
tonyqiu123 wants to merge 7 commits intomainfrom
W25/tony/deprecate-oAuth

Conversation

@tonyqiu123
Copy link
Contributor

@tonyqiu123 tonyqiu123 commented Feb 4, 2025 •
edited
Loading

Notion ticket link

Deprecate OAuth

Implementation description

Steps to test

  1. Signup and login in all ways. I am blocked on the 'postgres' database not found issue so I couldn't follow instructions of the proper admin signup and login flow to provide proper details of testing steps.

What should reviewers focus on?

  • Check if all signup and login works properly!

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

shencynthia
shencynthia requested changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@shencynthia shencynthia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround time with this one, I left a comment that requires your review, but otherwise looks good :)

shencynthia
shencynthia reviewed Feb 14, 2025
View reviewed changes
Copy link
Contributor

@shencynthia shencynthia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you run the linter, the checks are failing. Thanks!

@shencynthia
Copy link
Contributor

shencynthia commented Feb 14, 2025

https://github.com/uwblueprint/humane-society?tab=readme-ov-file#linting--formatting

vibbyg
vibbyg reviewed Feb 21, 2025
View reviewed changes
frontend/src/APIClients/AuthAPIClient.ts
Copy link
Contributor

@vibbyg vibbyg Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the console.log() here?

vibbyg
vibbyg approved these changes Feb 21, 2025
View reviewed changes
Copy link
Contributor

@vibbyg vibbyg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me !
There are a few linting warnings present if you want to take a second to look at those.

@shencynthia shencynthia marked this pull request as draft March 4, 2025 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vibbyg vibbyg vibbyg approved these changes

@shencynthia shencynthia Awaiting requested review from shencynthia

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tonyqiu123 @shencynthia @vibbyg